Skip to content

Refactor RunningFunctionTaskMap to use composite string key#4958

Open
nturinski wants to merge 1 commit intomainfrom
nat/refactor-running-func-task-map
Open

Refactor RunningFunctionTaskMap to use composite string key#4958
nturinski wants to merge 1 commit intomainfrom
nat/refactor-running-func-task-map

Conversation

@nturinski
Copy link
Copy Markdown
Member

Summary

Replaces Map<WorkspaceFolder|TaskScope, IRunningFuncTask[]> with Map<string, IRunningFuncTask> using a composite key derived from folder.uri + normalized cwd. This eliminates the linear array search in .get() and moves the ${workspaceFolder} path resolution out of the map class into a shared resolveAndNormalizeCwd() utility.

Also removes TaskScope from all type signatures since FuncTaskProvider already filters out Global/Workspace scoped tasks, making TaskScope dead code across the codebase.

Changes

Core (funcHostTask.ts)

  • RunningFunctionTaskMap — internal map is now Map<string, IRunningFuncTask> with a composite key (folder.uri.toString() + "::" + normalizedCwd). No more array storage or linear search.
  • resolveAndNormalizeCwd() — new exported utility that handles ${workspaceFolder} replacement + normalizePath(). All callers use this before interacting with the map.
  • .set(folder, normalizedCwd, task) — takes normalized cwd as an explicit parameter.
  • .get()/.has() — direct O(1) map lookups instead of Array.find().
  • .delete(folder, cwd?) / .getAll(folder) — prefix-match for killAll and tree-view cases.
  • IStoppedFuncTask.workspaceFolder, IRunningFuncTaskWithScope.scope — narrowed from WorkspaceFolder | TaskScope to WorkspaceFolder.
  • stopFuncTaskIfRunning(), killFuncProcessByPortOrPid(), getFuncPortFromTaskOrProject() — narrowed parameter types.
  • Event handlers now include a defensive typeof scope === 'object' guard and normalize cwd before all map operations.

Dependent files (8 files)

  • funcHostDebugUtils.tsgetScopeLabel() narrowed, removed dead 'Global' branch.
  • HostTaskNode.ts — narrowed constructor, normalizes cwd before .get().
  • HostErrorNode.ts — narrowed constructor param.
  • registerFunctionHostDebugView.ts — normalizes cwd before all .get() calls.
  • pickFuncProcess.ts — normalizes buildPath before .has() and .get().
  • LocalProject.ts — now passes effectiveProjectPath (always available) for more correct map lookups instead of relying on no-arg fallback.
  • listLocalFunctions.ts — normalizes effectiveProjectPath before .has().
  • LocalProjectTreeItem.ts — removed TaskScope import, narrowed onFuncTaskChanged.

Testing

  • npm run build:check passes with zero type errors.
  • All unit tests pass.

Replace Map<WorkspaceFolder|TaskScope, IRunningFuncTask[]> with
Map<string, IRunningFuncTask> using a composite key derived from
folder.uri + normalized cwd. This eliminates the linear array search
in .get() and moves the  path resolution out of
the map class into a shared resolveAndNormalizeCwd() utility.

Also removes TaskScope from all type signatures since FuncTaskProvider
already filters out Global/Workspace scoped tasks, making TaskScope
dead code across the codebase. A defensive typeof scope === 'object'
guard is added in the event handler for safety.

- LocalProject.ts now passes effectiveProjectPath (always available)
  for more correct map lookups instead of relying on no-arg fallback
- pickFuncProcess.ts normalizes buildPath before .has()/.get() calls
- All debug view nodes narrow workspaceFolder to WorkspaceFolder
Copilot AI review requested due to automatic review settings April 3, 2026 00:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors function-host task tracking to use a composite string key (workspaceFolder URI + normalized cwd) so running task lookup is O(1) and ${workspaceFolder}/cwd normalization is centralized in a new utility.

Changes:

  • Replaced Map<WorkspaceFolder|TaskScope, IRunningFuncTask[]> with Map<string, IRunningFuncTask> keyed by folderUri + "::" + normalizedCwd.
  • Added resolveAndNormalizeCwd() and updated call sites to normalize cwd/build paths prior to map operations.
  • Narrowed multiple APIs/events/interfaces to use WorkspaceFolder only (removing TaskScope).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/funcCoreTools/funcHostTask.ts Implements the composite-key map, adds resolveAndNormalizeCwd, and narrows scope types.
src/workspace/LocalProject.ts Uses normalized effective project path for running host port lookup.
src/workspace/listLocalFunctions.ts Uses normalized effective project path when checking if host is running.
src/tree/localProject/LocalProjectTreeItem.ts Narrows func-task event scope typing to WorkspaceFolder.
src/debug/registerFunctionHostDebugView.ts Normalizes cwd before map lookups for debug view actions.
src/debug/nodes/HostTaskNode.ts Normalizes cwd before reading task details/errors from the map.
src/debug/nodes/HostErrorNode.ts Narrows stored scope to WorkspaceFolder.
src/debug/nodes/funcHostDebugUtils.ts Removes dead Global scope label branch; WorkspaceFolder only.
src/commands/pickFuncProcess.ts Normalizes build path for has/get checks against the new map.

Comment on lines +69 to +76
* Resolves a raw cwd string by replacing `${workspaceFolder}` with the actual workspace folder path,
* then normalizes the result. Callers should use this before interacting with `runningFuncTaskMap`.
*/
export function resolveAndNormalizeCwd(folder: vscode.WorkspaceFolder, rawCwd?: string): string | undefined {
if (!rawCwd) {
return undefined;
}
const resolved = rawCwd.replace('${workspaceFolder}', folder.uri.path);
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveAndNormalizeCwd replaces ${workspaceFolder} using folder.uri.path, which is a URI path (posix/encoded) and can diverge from filesystem paths (especially on Windows). This can cause the normalized cwd used as the map key to differ between task-start (from ${workspaceFolder}) and lookups (often passed as *.uri.fsPath), leading to missed get/has/delete matches. Use folder.uri.fsPath (and ideally a global replacement) to keep everything in filesystem-path space before normalization.

Suggested change
* Resolves a raw cwd string by replacing `${workspaceFolder}` with the actual workspace folder path,
* then normalizes the result. Callers should use this before interacting with `runningFuncTaskMap`.
*/
export function resolveAndNormalizeCwd(folder: vscode.WorkspaceFolder, rawCwd?: string): string | undefined {
if (!rawCwd) {
return undefined;
}
const resolved = rawCwd.replace('${workspaceFolder}', folder.uri.path);
* Resolves a raw cwd string by replacing all `${workspaceFolder}` tokens with the actual workspace folder filesystem path,
* then normalizes the result. Callers should use this before interacting with `runningFuncTaskMap`.
*/
export function resolveAndNormalizeCwd(folder: vscode.WorkspaceFolder, rawCwd?: string): string | undefined {
if (!rawCwd) {
return undefined;
}
const resolved = rawCwd.split('${workspaceFolder}').join(folder.uri.fsPath);

Copilot uses AI. Check for mistakes.

if (buildPath) {
runningFuncTaskMap.delete(workspaceFolder, buildPath);
if (normalizedBuildPath) {
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In stopFuncTaskIfRunning, when buildPath is omitted normalizedBuildPath is undefined, but the map entry is never deleted (if (normalizedBuildPath) ...). This leaves stale entries for hosts started with the default cwd (no explicit options.cwd), causing runningFuncTaskMap.has/get to think the host is still running. Consider deleting the folder-only key when normalizedBuildPath is undefined (or always delete the specific key you looked up).

Suggested change
if (normalizedBuildPath) {
if (!killAll) {

Copilot uses AI. Check for mistakes.
Comment on lines +115 to 126
public delete(folder: vscode.WorkspaceFolder, normalizedCwd?: string): void {
if (normalizedCwd !== undefined) {
this._map.delete(this.makeKey(folder, normalizedCwd));
} else {
// Delete all entries for this folder
const prefix = this.folderPrefix(folder);
for (const key of [...this._map.keys()]) {
if (key === prefix || key.startsWith(`${prefix}::`)) {
this._map.delete(key);
}
}
}
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RunningFunctionTaskMap.delete(folder, normalizedCwd?) treats normalizedCwd === undefined as “delete all entries for this folder”. That makes it impossible to delete a single entry whose normalized cwd is actually undefined (i.e., tasks that rely on the default cwd). In the current code, onDidEndTaskProcess calls runningFuncTaskMap.delete(scope, normalizedCwd) where normalizedCwd can be undefined, which would wipe all running tasks for that folder. Consider separating the APIs (e.g., deleteOne(folder, cwdKey) vs deleteAll(folder)), or using a sentinel value for the default-cwd key so single-entry deletion is always possible.

Copilot uses AI. Check for mistakes.
@nturinski nturinski marked this pull request as ready for review April 13, 2026 21:32
@nturinski nturinski requested a review from a team as a code owner April 13, 2026 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants